Skip to content

Conversation

@qinkunbao
Copy link
Member

@qinkunbao qinkunbao commented May 27, 2025

Accoring to the discussion in #140529, we need to SSCL can be
created from multiple ignore list files, so we can repeat-fsanitize-ignorelist=. The change is necessary to achieve the feature described in #139128.

qinkunbao added 2 commits May 27, 2025 02:36
Created using spr 1.3.6
@llvmbot
Copy link
Member

llvmbot commented May 27, 2025

@llvm/pr-subscribers-llvm-support

Author: Qinkun Bao (qinkunbao)

Changes

Accoring to the discussion in
#140529, we need to SSCL can be
created from multiple ignore list files, so we can repeat
-fsanitize-ignorelist=. The change is necessary to achieve the feature
described in #139128.


Full diff: https://github.com/llvm/llvm-project/pull/141540.diff

4 Files Affected:

  • (modified) llvm/include/llvm/Support/SpecialCaseList.h (+9-6)
  • (modified) llvm/lib/Support/SpecialCaseList.cpp (+10-6)
  • (modified) llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp (+9-6)
  • (modified) llvm/unittests/Support/SpecialCaseListTest.cpp (+45-17)
diff --git a/llvm/include/llvm/Support/SpecialCaseList.h b/llvm/include/llvm/Support/SpecialCaseList.h
index 653a3b14ebf03..bce337f553a93 100644
--- a/llvm/include/llvm/Support/SpecialCaseList.h
+++ b/llvm/include/llvm/Support/SpecialCaseList.h
@@ -17,6 +17,7 @@
 #include "llvm/Support/GlobPattern.h"
 #include "llvm/Support/Regex.h"
 #include <memory>
+#include <utility>
 #include <string>
 #include <vector>
 
@@ -93,17 +94,17 @@ class SpecialCaseList {
   LLVM_ABI bool inSection(StringRef Section, StringRef Prefix, StringRef Query,
                           StringRef Category = StringRef()) const;
 
-  /// Returns the line number corresponding to the special case list entry if
-  /// the special case list contains a line
+  /// Returns the file index and the line numebr <FileIdx, LineNo> corresponding
+  /// to the special case list entry if the special case list contains a line
   /// \code
   ///   @Prefix:<E>=@Category
   /// \endcode
   /// where @Query satisfies the glob <E> in a given @Section.
-  /// Returns zero if there is no exclusion entry corresponding to this
+  /// Returns (zero, zero) if there is no exclusion entry corresponding to this
   /// expression.
-  LLVM_ABI unsigned inSectionBlame(StringRef Section, StringRef Prefix,
-                                   StringRef Query,
-                                   StringRef Category = StringRef()) const;
+  LLVM_ABI std::pair<unsigned, unsigned>
+  inSectionBlame(StringRef Section, StringRef Prefix, StringRef Query,
+                 StringRef Category = StringRef()) const;
 
 protected:
   // Implementations of the create*() functions that can also be used by derived
@@ -145,12 +146,14 @@ class SpecialCaseList {
     Section(std::unique_ptr<Matcher> M) : SectionMatcher(std::move(M)) {};
     Section() : Section(std::make_unique<Matcher>()) {};
 
+    unsigned FileIdx;
     std::unique_ptr<Matcher> SectionMatcher;
     SectionEntries Entries;
     std::string SectionStr;
   };
 
   std::vector<Section> Sections;
+  unsigned currFileIdx;
 
   LLVM_ABI Expected<Section *> addSection(StringRef SectionStr, unsigned LineNo,
                                           bool UseGlobs = true);
diff --git a/llvm/lib/Support/SpecialCaseList.cpp b/llvm/lib/Support/SpecialCaseList.cpp
index 56327b56282c2..5a66f163d7913 100644
--- a/llvm/lib/Support/SpecialCaseList.cpp
+++ b/llvm/lib/Support/SpecialCaseList.cpp
@@ -112,6 +112,7 @@ bool SpecialCaseList::createInternal(const std::vector<std::string> &Paths,
       return false;
     }
     std::string ParseError;
+    ++currFileIdx;
     if (!parse(FileOrErr.get().get(), ParseError)) {
       Error = (Twine("error parsing file '") + Path + "': " + ParseError).str();
       return false;
@@ -122,6 +123,7 @@ bool SpecialCaseList::createInternal(const std::vector<std::string> &Paths,
 
 bool SpecialCaseList::createInternal(const MemoryBuffer *MB,
                                      std::string &Error) {
+  ++currFileIdx;
   if (!parse(MB, Error))
     return false;
   return true;
@@ -133,6 +135,7 @@ SpecialCaseList::addSection(StringRef SectionStr, unsigned LineNo,
   Sections.emplace_back();
   auto &Section = Sections.back();
   Section.SectionStr = SectionStr;
+  Section.FileIdx = currFileIdx;
 
   if (auto Err = Section.SectionMatcher->insert(SectionStr, LineNo, UseGlobs)) {
     return createStringError(errc::invalid_argument,
@@ -207,20 +210,21 @@ SpecialCaseList::~SpecialCaseList() = default;
 
 bool SpecialCaseList::inSection(StringRef Section, StringRef Prefix,
                                 StringRef Query, StringRef Category) const {
-  return inSectionBlame(Section, Prefix, Query, Category);
+  auto [FileIdx, LineNo] = inSectionBlame(Section, Prefix, Query, Category);
+  return LineNo;
 }
 
-unsigned SpecialCaseList::inSectionBlame(StringRef Section, StringRef Prefix,
-                                         StringRef Query,
-                                         StringRef Category) const {
+std::pair<unsigned, unsigned>
+SpecialCaseList::inSectionBlame(StringRef Section, StringRef Prefix,
+                                StringRef Query, StringRef Category) const {
   for (auto it = Sections.crbegin(); it != Sections.crend(); ++it) {
     if (it->SectionMatcher->match(Section)) {
       unsigned Blame = inSectionBlame(it->Entries, Prefix, Query, Category);
       if (Blame)
-        return Blame;
+        return {it->FileIdx, Blame};
     }
   }
-  return 0;
+  return {0, 0};
 }
 
 unsigned SpecialCaseList::inSectionBlame(const SectionEntries &Entries,
diff --git a/llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp b/llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
index 0f2c7da94230e..b372957204691 100644
--- a/llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
+++ b/llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
@@ -78,8 +78,7 @@ static void printBlameContext(const DILineInfo &LineInfo, unsigned Context) {
   File->getBuffer().split(Lines, '\n');
 
   for (unsigned i = std::max<size_t>(1, LineInfo.Line - Context);
-       i <
-       std::min<size_t>(Lines.size() + 1, LineInfo.Line + Context + 1);
+       i < std::min<size_t>(Lines.size() + 1, LineInfo.Line + Context + 1);
        ++i) {
     if (i == LineInfo.Line)
       outs() << ">";
@@ -193,12 +192,16 @@ printIndirectCFInstructions(FileAnalysis &Analysis,
 
     unsigned BlameLine = 0;
     for (auto &K : {"cfi-icall", "cfi-vcall"}) {
-      if (!BlameLine)
-        BlameLine =
+      if (!BlameLine) {
+        auto [FileIdx, Line] =
             SpecialCaseList->inSectionBlame(K, "src", LineInfo.FileName);
-      if (!BlameLine)
-        BlameLine =
+        BlameLine = Line;
+      }
+      if (!BlameLine) {
+        auto [FileIdx, Line] =
             SpecialCaseList->inSectionBlame(K, "fun", LineInfo.FunctionName);
+        BlameLine = Line;
+      }
     }
 
     if (BlameLine) {
diff --git a/llvm/unittests/Support/SpecialCaseListTest.cpp b/llvm/unittests/Support/SpecialCaseListTest.cpp
index ce58328f396b6..da4c557e740e6 100644
--- a/llvm/unittests/Support/SpecialCaseListTest.cpp
+++ b/llvm/unittests/Support/SpecialCaseListTest.cpp
@@ -14,6 +14,7 @@
 #include "gtest/gtest.h"
 
 using testing::HasSubstr;
+using testing::Pair;
 using testing::StartsWith;
 using namespace llvm;
 
@@ -70,13 +71,14 @@ TEST_F(SpecialCaseListTest, Basic) {
   EXPECT_FALSE(SCL->inSection("", "fun", "hello"));
   EXPECT_FALSE(SCL->inSection("", "src", "hello", "category"));
 
-  EXPECT_EQ(3u, SCL->inSectionBlame("", "src", "hello"));
-  EXPECT_EQ(4u, SCL->inSectionBlame("", "src", "bye"));
-  EXPECT_EQ(5u, SCL->inSectionBlame("", "src", "hi", "category"));
-  EXPECT_EQ(6u, SCL->inSectionBlame("", "src", "zzzz", "category"));
-  EXPECT_EQ(0u, SCL->inSectionBlame("", "src", "hi"));
-  EXPECT_EQ(0u, SCL->inSectionBlame("", "fun", "hello"));
-  EXPECT_EQ(0u, SCL->inSectionBlame("", "src", "hello", "category"));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "hello"), Pair(1u, 3u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "bye"), Pair(1u, 4u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "hi", "category"), Pair(1u, 5u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "zzzz", "category"), Pair(1u, 6u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "hi"), Pair(0u, 0u));
+  EXPECT_THAT(SCL->inSectionBlame("", "fun", "hello"), Pair(0u, 0u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "hello", "category"),
+              Pair(0u, 0u));
 }
 
 TEST_F(SpecialCaseListTest, CorrectErrorLineNumberWithBlankLine) {
@@ -216,8 +218,9 @@ TEST_F(SpecialCaseListTest, NoTrigramsInARule) {
 }
 
 TEST_F(SpecialCaseListTest, RepetitiveRule) {
-  std::unique_ptr<SpecialCaseList> SCL = makeSpecialCaseList("fun:*bar*bar*bar*bar*\n"
-                                                             "fun:bar*\n");
+  std::unique_ptr<SpecialCaseList> SCL =
+      makeSpecialCaseList("fun:*bar*bar*bar*bar*\n"
+                          "fun:bar*\n");
   EXPECT_TRUE(SCL->inSection("", "fun", "bara"));
   EXPECT_FALSE(SCL->inSection("", "fun", "abara"));
   EXPECT_TRUE(SCL->inSection("", "fun", "barbarbarbar"));
@@ -226,7 +229,8 @@ TEST_F(SpecialCaseListTest, RepetitiveRule) {
 }
 
 TEST_F(SpecialCaseListTest, SpecialSymbolRule) {
-  std::unique_ptr<SpecialCaseList> SCL = makeSpecialCaseList("src:*c\\+\\+abi*\n");
+  std::unique_ptr<SpecialCaseList> SCL =
+      makeSpecialCaseList("src:*c\\+\\+abi*\n");
   EXPECT_TRUE(SCL->inSection("", "src", "c++abi"));
   EXPECT_FALSE(SCL->inSection("", "src", "c\\+\\+abi"));
 }
@@ -242,8 +246,9 @@ TEST_F(SpecialCaseListTest, PopularTrigram) {
 }
 
 TEST_F(SpecialCaseListTest, EscapedSymbols) {
-  std::unique_ptr<SpecialCaseList> SCL = makeSpecialCaseList("src:*c\\+\\+abi*\n"
-                                                             "src:*hello\\\\world*\n");
+  std::unique_ptr<SpecialCaseList> SCL =
+      makeSpecialCaseList("src:*c\\+\\+abi*\n"
+                          "src:*hello\\\\world*\n");
   EXPECT_TRUE(SCL->inSection("", "src", "dir/c++abi"));
   EXPECT_FALSE(SCL->inSection("", "src", "dir/c\\+\\+abi"));
   EXPECT_FALSE(SCL->inSection("", "src", "c\\+\\+abi"));
@@ -323,10 +328,33 @@ TEST_F(SpecialCaseListTest, Version3) {
   EXPECT_TRUE(SCL->inSection("sect2", "src", "def"));
   EXPECT_TRUE(SCL->inSection("sect1", "src", "def"));
 
-  EXPECT_EQ(2u, SCL->inSectionBlame("sect1", "src", "fooz"));
-  EXPECT_EQ(4u, SCL->inSectionBlame("sect1", "src", "barz"));
-  EXPECT_EQ(5u, SCL->inSectionBlame("sect1", "src", "def"));
-  EXPECT_EQ(8u, SCL->inSectionBlame("sect2", "src", "def"));
-  EXPECT_EQ(8u, SCL->inSectionBlame("sect2", "src", "dez"));
+  EXPECT_THAT(SCL->inSectionBlame("sect1", "src", "fooz"), Pair(1u, 2u));
+  EXPECT_THAT(SCL->inSectionBlame("sect1", "src", "barz"), Pair(1u, 4u));
+  EXPECT_THAT(SCL->inSectionBlame("sect1", "src", "def"), Pair(1u, 5u));
+  EXPECT_THAT(SCL->inSectionBlame("sect2", "src", "def"), Pair(1u, 8u));
+  EXPECT_THAT(SCL->inSectionBlame("sect2", "src", "dez"), Pair(1u, 8u));
 }
+
+TEST_F(SpecialCaseListTest, FileIdx) {
+  std::vector<std::string> Files;
+  Files.push_back(makeSpecialCaseListFile("src:bar\n"
+                                          "src:*foo*\n"
+                                          "src:ban=init\n"
+                                          "src:baz\n"
+                                          "src:*def\n"));
+  Files.push_back(makeSpecialCaseListFile("src:baz\n"
+                                          "src:car\n"
+                                          "src:def*"));
+  auto SCL = SpecialCaseList::createOrDie(Files, *vfs::getRealFileSystem());
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "bar"), Pair(1u, 1u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "fooaaaaaa"), Pair(1u, 2u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "ban", "init"), Pair(1u, 3u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "baz"), Pair(2u, 1u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "car"), Pair(2u, 2u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "aaaadef"), Pair(1u, 5u));
+  EXPECT_THAT(SCL->inSectionBlame("", "src", "defaaaaa"), Pair(2u, 3u));
+  for (auto &Path : Files)
+    sys::fs::remove(Path);
 }
+
+} // namespace

@github-actions
Copy link

github-actions bot commented May 27, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Created using spr 1.3.6
Created using spr 1.3.6
@qinkunbao qinkunbao changed the title Change inSectionBlame to return pair<uint, uint> (FileIdx, LineNo). Change SpecialCaseList::inSectionBlame to return pair<uint, uint> (FileIdx, LineNo). May 27, 2025
@qinkunbao qinkunbao changed the title Change SpecialCaseList::inSectionBlame to return pair<uint, uint> (FileIdx, LineNo). [NFCI] Change SpecialCaseList::inSectionBlame to return pair<uint, uint> (FileIdx, LineNo). May 27, 2025
@qinkunbao qinkunbao requested a review from vitalybuka May 27, 2025 17:43
};

std::vector<Section> Sections;
unsigned currFileIdx;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this as part of the state

please add another argument into addSection(fileIdx) and parse(fileIdx)


bool SpecialCaseList::createInternal(const MemoryBuffer *MB,
std::string &Error) {
++currFileIdx;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MemoryBuffer is always one, let's make it 0

return false;
}
std::string ParseError;
++currFileIdx;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouild be nice to match idx in Paths array

for (const auto &Path : Paths) -> for (unsigned Idx = 0; ....)

@qinkunbao qinkunbao changed the base branch from users/qinkunbao/spr/main.change-insectionblame-to-return-pairuint-uint-fileidx-lineno to main May 28, 2025 02:56
@vitalybuka
Copy link
Collaborator

I have local changes, I'll push update

Created using spr 1.3.6
Copy link
Collaborator

@vitalybuka vitalybuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please double check and merge

qinkunbao added 2 commits May 27, 2025 20:11
Created using spr 1.3.6
Created using spr 1.3.6
class SpecialCaseList {
public:
constexpr static std::pair<unsigned, unsigned> NotFound = {0, 0};
static constexpr std::pair<unsigned, unsigned> NotFound = {0, 0};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought static constexpr and constexpr static mean exactly the same thing

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, for consistency

@qinkunbao qinkunbao merged commit 81bde10 into main May 28, 2025
8 of 10 checks passed
@qinkunbao qinkunbao deleted the users/qinkunbao/spr/change-insectionblame-to-return-pairuint-uint-fileidx-lineno branch May 28, 2025 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants